Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose LocalisationManager.GetLocalisedString() as public #6377

Merged

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Sep 19, 2024

The initial rationale for it being internal was likely that there was never going to be any meaningful external usage of it, but as it turns out, there are cases where you want this, one of them being ppy/osu#29913 - without having the non-bindable variant exposed, weird games of taking out the bindable and then manually unbinding it immediately after use have to be played to avoid reference leaks.

The pre-existing xmldoc should do the job when it comes to warning users that the method will likely not do what they want it to in most cases.

The initial rationale for it being internal was likely that there was
never going to be any meaningful external usage of it, but as it turns
out, there are cases where you want this, one of them being
ppy/osu#29913 - without having the non-bindable
variant exposed, weird games of taking out the bindable and then
manually unbinding it immediately after use have to be played to avoid
reference leaks.

The pre-existing xmldoc should do the job when it comes to warning users
that the method will likely not do what they want it to in most cases.
@peppy peppy merged commit 23e6932 into ppy:master Sep 19, 2024
19 of 21 checks passed
@bdach bdach deleted the localisation-manager-localised-non-bindable-public branch September 19, 2024 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants